Skip to content

Adding enigma2 media player#21271

Merged
rytilahti merged 28 commits into
home-assistant:devfrom
fbradyirl:enigma2Take2
Mar 8, 2019
Merged

Adding enigma2 media player#21271
rytilahti merged 28 commits into
home-assistant:devfrom
fbradyirl:enigma2Take2

Conversation

@fbradyirl
Copy link
Copy Markdown
Contributor

@fbradyirl fbradyirl commented Feb 21, 2019

Description:

This PR adds support for Enigma2 based set top box receivers. Examples include devices from Vu (Duo, Duo2, Uno, etc) and lots of manufacturers who support the open sourced images based on Enigma2, such as OpenVix and Black Hole.

At present, the platform supports:

  1. Power standby/wake toggle (on/off)
  2. Select next/prev channel
  3. Volume set
  4. Mute/unmute
  5. Query current program info such as:
    • TV station active,
    • Programme Name & Description
    • Picon (channel icon) for the station
  6. Auto-discover of enigma2 boxes (via netdisco - Pull request in netdisco:** Add support for discovering enigma2 based devices home-assistant-libs/netdisco#241 )

How does it work?

Home Assistant Enigma2 Architecture
)

note: Component should be worded 'Platform' in the diagram

This PR contains a media_player platform named enigma2 for Home Assistant, which allows basic control of the unit and the TV via my 3rd party pip module, called openwebifpy.

e2v2

More details and example automations can be found here.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8776

Discovery

Pull request in netdisco: home-assistant-libs/netdisco#241

Example entry for configuration.yaml (if applicable):

# Example configuration.yaml entry
media_player:
  - platform: enigma2
    host: 192.168.1.12
    name: Vu Duo2

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
@fbradyirl fbradyirl requested a review from a team as a code owner February 22, 2019 12:13
@fbradyirl
Copy link
Copy Markdown
Contributor Author

cc @cinzas @KavajNaruj (I know you guys previously worked on enigma2 PRs).

Comment thread homeassistant/components/media_player/enigma2.py Outdated
Comment thread homeassistant/components/media_player/enigma2.py Outdated
@cinzas
Copy link
Copy Markdown

cinzas commented Feb 28, 2019

Hi, I had my PR closed, maybe I will reopen it since I've now changed enigma2 to a platform that implements media_player and notify.

Let me know if you PR gets accepted.

With my component you have a little more features

Nevertheless, great work :)

Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
@fbradyirl
Copy link
Copy Markdown
Contributor Author

cc @mihalski for review (if you have time!)

@fbradyirl
Copy link
Copy Markdown
Contributor Author

@balloob I think all your comments should be addressed now. Let me know if there's anything else you need. Thanks.

@adrianmihalko
Copy link
Copy Markdown

Wow, thank you. I hope it will be merged soon.

Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Copy link
Copy Markdown
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading your comment wrt. adding API to the upstream lib, I added a couple of things that could be done nicer inside the lib to provide a cleaner interface for homeassistant.

Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
@fbradyirl
Copy link
Copy Markdown
Contributor Author

fbradyirl commented Mar 6, 2019

Thank you @rytilahti for your review. I do agree with all your comments. Most of those things should be encapsulated inside the module and I’ll do that tomorrow.

@fbradyirl fbradyirl closed this Mar 7, 2019
@fbradyirl fbradyirl reopened this Mar 7, 2019
@ghost ghost added in progress and removed in progress labels Mar 7, 2019
Copy link
Copy Markdown
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good, I added a couple of nitpicks but I think this is ready to be merged!

Comment thread homeassistant/components/enigma2/__init__.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py Outdated
Comment thread homeassistant/components/enigma2/media_player.py
- rename prefer_picon to use_channel_icon
@rytilahti
Copy link
Copy Markdown
Member

Please fix the travis build, I'll do a quick review on the docs and I think this can be then merged! 🎉

also fix travis

also move setup out of init
@fbradyirl
Copy link
Copy Markdown
Contributor Author

Please fix the travis build, I'll do a quick review on the docs and I think this can be then merged! 🎉

Thank you @rytilahti ! I was actually just doing some work there to move the setup of the device outside of the __init__, so I hope you like that also.

I did some testing on discovery and non-discovery and it all works well.

Thanks again!

@rytilahti rytilahti merged commit 4571f1b into home-assistant:dev Mar 8, 2019
@ghost ghost removed the in progress label Mar 8, 2019
@fbradyirl fbradyirl deleted the enigma2Take2 branch March 8, 2019 13:59
@fbradyirl
Copy link
Copy Markdown
Contributor Author

Note: for auto-discovery to work for this platform, we will need a bump of the Netdisco library since this PR got merged.

home-assistant-libs/netdisco#241

cc @balloob

SERVICE_HASSIO: ('hassio', None),
SERVICE_AXIS: ('axis', None),
SERVICE_APPLE_TV: ('apple_tv', None),
SERVICE_ENIGMA2: ('enigma2', None),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inserting media_player platform as first item in the tuple would load that platform directly instead of loading the component and firing the event.

"""List of available input sources."""
return self.e2_box.source_list

@asyncio.coroutine
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use async def instead of coroutine decorator.

@asyncio.coroutine
def async_select_source(self, source):
"""Select input source."""
self.e2_box.select_source(self.e2_box.sources[source])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this async safe?

attributes[ATTR_MEDIA_DESCRIPTION] = \
self.e2_box.status_info['currservice_fulldescription']
attributes[ATTR_MEDIA_START_TIME] = \
self.e2_box.status_info['currservice_begin']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these times absolute utc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants